gh-90005: Port readline and curses to PY_STDLIB_MOD (GH-94452)#94452
gh-90005: Port readline and curses to PY_STDLIB_MOD (GH-94452)#94452tiran merged 7 commits intopython:mainfrom
Conversation
|
🤖 New build scheduled with the buildbot fleet by @tiran for commit 33d9e296124d7daeef89fca957bdab51e1e1d11b 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
8a61447 to
dd59373
Compare
|
🤖 New build scheduled with the buildbot fleet by @tiran for commit dd5937322e181781c03213621eddb0fb88527c48 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
erlend-aasland
left a comment
There was a problem hiding this comment.
Great work! I left some comments.
configure.ac
Outdated
There was a problem hiding this comment.
Looks like this will create havoc for WASM/Emscripten static builds; multiple -l switches are passed to the compiler:
config.log:
29899 configure:21034: checking for rl_pre_input_hook in -lreadline
29900 configure:21059: gcc -o conftest conftest.c -lreadline -lreadline -lintl -ldl >&5
29901 configure:21059: $? = 0
29902 configure:21069: result: yes
29903 configure:21080: checking for rl_completion_display_matches_hook in -lreadline
29904 configure:21105: gcc -o conftest conftest.c -lreadline -lreadline -lintl -ldl >&5
29905 configure:21105: $? = 0
29906 configure:21115: result: yes
29907 configure:21126: checking for rl_resize_terminal in -lreadline
29908 configure:21151: gcc -o conftest conftest.c -lreadline -lreadline -lintl -ldl >&5
There was a problem hiding this comment.
Good point, should I place a WITH_SAVE_ENV around each check? By the way, readline is not yet supported on Emscripten.
There was a problem hiding this comment.
WITH_SAVE_ENV can't be nested :( But your LIBS_SAVE workaround should work well.
There was a problem hiding this comment.
Unfortunately, it does not work. They're still there.
There was a problem hiding this comment.
AC_CHECK_LIB might be adding the library before running the check.
There was a problem hiding this comment.
Yeah, we need a different approach here.
There was a problem hiding this comment.
Yeah. This reminds me I've got an open PR for the exact same issue for sqlite3 dependency detection :) I've tried various approaches, but I haven't found a neat way to solve it yet. (I haven't looked at it for a week either, though.)
dd59373 to
a72b632
Compare
erlend-aasland
left a comment
There was a problem hiding this comment.
These suggestions should work. You should be able to apply them as a batch.
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@protonmail.com>
|
@erlend-aasland I have refactored the code. There is a bit of repetition but IMHO it's ok for a few lookups. We could add our own variant of |
|
I have merged the PR. It works on all tested platforms. We can review and address the general issue with |
|
The change triggered a reference leak on Fedora: #94644 |
|
Hi, I think I found an issue. On Solaris, even a simple code like: char readline ();
int main (void) {
return readline ();
}needs to be linked with both Is there something I am missing or something I can try? |
|
The problem should be fixed by GH-94802. |
|
Ah, it's indeed fixed. I am sorry, I thought I am looking into latest main but it was 12 days old (when this was pushed). Thanks for help! |
Uh oh!
There was an error while loading. Please reload this page.